Skip to content

ext/iconv fix solaris/illumos which can support const/non const versi… #16619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ext/iconv fix solaris/illumos which can support const/non const versi… #16619

wants to merge 2 commits into from
+4 −1

Conversation

devnexen
Copy link
Member

…ons.

@devnexen
Copy link
Member Author

devnexen commented Nov 5, 2024

cc @petk would you mind trying this branch ? Thanks :)

@petk
Copy link
Member

petk commented Nov 6, 2024

I've checked it on Solaris 11.4 and it builds fine. But I'm not exactly sure what to test here. With or without this patch there are 9 failing tests in ext/iconv:

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Bug #52211 (iconv() returns part of string on error) [ext/iconv/tests/bug52211.phpt]
EUC-JP to ISO-2022-JP [ext/iconv/tests/eucjp2iso2022jp.phpt]
iconv_substr() with out of bounds offset [ext/iconv/tests/iconv_substr_out_of_bounds.phpt]
Bug #37773 (iconv_substr() gives "Unknown error" when string length = 1") [ext/iconv/tests/bug37773.phpt]
Test the basics to function iconv. [ext/iconv/tests/iconv_basic_001.phpt]
iconv() test 3 [ext/iconv/tests/iconv003.phpt]
Bug #69840 (iconv_substr() doesn't work with UTF-16BE) [ext/iconv/tests/bug69840.phpt]
iconv_strrpos() [ext/iconv/tests/iconv_strrpos.phpt]
iconv_mime_encode() [ext/iconv/tests/iconv_mime_encode.phpt]
=====================================================================

@devnexen
Copy link
Member Author

devnexen commented Nov 6, 2024

It is not resolving anything but a build warning concerning the iconv api itself.

e.g.

			if (iconv(cd, (**ICONV_CONST** char **)&in_p, &in_left, (char **) &out_p, &out_left) == (size_t)-1) {

@petk
Copy link
Member

petk commented Nov 6, 2024

With this patch applied I get more warnings:

Details 
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘_php_iconv_appendl’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:373:39: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
  373 |                         if (iconv(cd, (ICONV_CONST char **)&in_p, &in_left, (char **) &out_p, &out_left) == (size_t)-1) {
      |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                       |
      |                                       const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘php_iconv_string’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:469:36: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
  469 |                 result = iconv(cd, (ICONV_CONST char **) &in_p, &in_left, (char **) &out_p, &out_left);
      |                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                    |
      |                                    const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘_php_iconv_strlen’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:589:61: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
  589 |                 iconv(cd, more ? (ICONV_CONST char **)&in_p : NULL, more ? &in_left : NULL, (char **) &out_p, &out_left);
      |                                                             ^
      |                                                             |
      |                                                             const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘_php_iconv_substr’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:696:62: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
  696 |                 iconv(cd1, more ? (ICONV_CONST char **)&in_p : NULL, more ? &in_left : NULL, (char **) &out_p, &out_left);
      |                                                              ^
      |                                                              |
      |                                                              const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘_php_iconv_strpos’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:818:73: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
  818 |                 iconv_ret = iconv(cd, more ? (ICONV_CONST char **)&in_p : NULL, more ? &in_left : NULL, (char **) &out_p, &out_left);
      |                                                                         ^
      |                                                                         |
      |                                                                         const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘_php_iconv_mime_encode’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:1025:55: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
 1025 |                                         if (iconv(cd, (ICONV_CONST char **)&in_p, &in_left, (char **) &out_p, &out_left) == (size_t)-1) {
      |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       |
      |                                                       const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c:1109:55: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
 1109 |                                         if (iconv(cd, (ICONV_CONST char **)&in_p, &in_left, (char **) &out_p, &out_left) == (size_t)-1) {
      |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                       |
      |                                                       const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c: In function ‘php_iconv_stream_filter_append_bucket’:
/export/home/petk/projects/php-src/ext/iconv/iconv.c:2378:45: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
 2378 |                         if (iconv(self->cd, (ICONV_CONST char **)&pt, &tcnt, &pd, &ocnt) == (size_t)-1) {
      |                                             ^~~~~~~~~~~~~~~~~~~~~~~~
      |                                             |
      |                                             const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^
/export/home/petk/projects/php-src/ext/iconv/iconv.c:2444:57: warning: passing argument 2 of ‘iconv’ from incompatible pointer type [-Wincompatible-pointer-types]
 2444 |                                         iconv(self->cd, (ICONV_CONST char **)&ps, &icnt, &pd, &ocnt)) == (size_t)-1) {
      |                                                         ^~~~~~~~~~~~~~~~~~~~~~~~
      |                                                         |
      |                                                         const char **
In file included from /export/home/petk/projects/php-src/ext/iconv/iconv.c:36:
/usr/include/iconv.h:97:32: note: expected ‘char ** restrict’ but argument is of type ‘const char **’
   97 | extern size_t   iconv(iconv_t, char **_RESTRICT_KYWD,
      |                                ^

@devnexen
Copy link
Member Author

devnexen commented Nov 6, 2024

Thanks. do you have specific CFLAGS you pass for building ?

@petk
Copy link
Member

petk commented Nov 6, 2024

Everything by default:

./configure
gmake

@devnexen
Copy link
Member Author

devnexen commented Nov 6, 2024

Ok I ll look into it further thanks !

@NattyNarwhal NattyNarwhal mentioned this pull request Nov 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Extension: iconv
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@devnexen @petk